StTooltip: fix various warnings and use Clutter API correctly

Use ClutterContainer functions for adding the tooltip instead of
calling clutter_actor_set_parent behind the stage's back, and do
it inside st_widget_show_tooltip (which is a normal method) instead
of overriding st_tooltip_show, which is a vfunc and it is called
internally by Clutter, therefore it is limited in what it can safely
do.
Also, instead of positioning the tooltip with clutter_actor_set_position,
modify the anchor point when the associated widget moves, so that
only a redraw is queued.

https://bugzilla.gnome.org/show_bug.cgi?id=635100

diff --git a/src/st/st-tooltip.c b/src/st/st-tooltip.c
index 09e7aa76508ad5a078497a002aa0a60f2bfbf919..5ef0f0267ec046dbc54cb22aa4a1440634bf8aad 100644 (file)
--- a/src/st/st-tooltip.c
+++ b/src/st/st-tooltip.c
@@ -208,26 +208,6 @@ st_tooltip_paint (ClutterActor *self)
   clutter_actor_paint (CLUTTER_ACTOR (priv->label));
 }
 
-static void
-st_tooltip_map (ClutterActor *self)
-{
-  StTooltipPrivate *priv = ST_TOOLTIP (self)->priv;
-
-  CLUTTER_ACTOR_CLASS (st_tooltip_parent_class)->map (self);
-
-  clutter_actor_map (CLUTTER_ACTOR (priv->label));
-}
-
-static void
-st_tooltip_unmap (ClutterActor *self)
-{
-  StTooltipPrivate *priv = ST_TOOLTIP (self)->priv;
-
-  CLUTTER_ACTOR_CLASS (st_tooltip_parent_class)->unmap (self);
-
-  clutter_actor_unmap (CLUTTER_ACTOR (priv->label));
-}
-
 static void
 st_tooltip_dispose (GObject *self)
 {
@@ -259,8 +239,6 @@ st_tooltip_class_init (StTooltipClass *klass)
   actor_class->get_preferred_height = st_tooltip_get_preferred_height;
   actor_class->allocate = st_tooltip_allocate;
   actor_class->paint = st_tooltip_paint;
-  actor_class->map = st_tooltip_map;
-  actor_class->unmap = st_tooltip_unmap;
   actor_class->show = st_tooltip_show;
   actor_class->show_all = st_tooltip_show_all;
   actor_class->hide_all = st_tooltip_hide_all;
@@ -302,16 +280,13 @@ st_tooltip_update_position (StTooltip *tooltip)
   StTooltipPrivate *priv = tooltip->priv;
   ClutterGeometry *tip_area = tooltip->priv->tip_area;
   gfloat tooltip_w, tooltip_h, tooltip_x, tooltip_y;
-  gfloat stage_w, stage_h;
-  ClutterActor *stage;
-
-  /* ensure the tooltip with is not fixed size */
-  clutter_actor_set_size ((ClutterActor*) tooltip, -1, -1);
+  gfloat parent_w, parent_h;
+  ClutterActor *parent;
 
   /* if no area set, just position ourselves top left */
   if (!priv->tip_area)
     {
-      clutter_actor_set_position ((ClutterActor*) tooltip, 0, 0);
+      clutter_actor_set_anchor_point ((ClutterActor*) tooltip, 0, 0);
       return;
     }
 
@@ -326,36 +301,28 @@ st_tooltip_update_position (StTooltip *tooltip)
   tooltip_x = (int)(tip_area->x + (tip_area->width / 2) - (tooltip_w / 2));
   tooltip_y = (int)(tip_area->y + tip_area->height);
 
-  stage = clutter_actor_get_stage ((ClutterActor *) tooltip);
-  if (!stage)
+  parent = clutter_actor_get_parent ((ClutterActor *) tooltip);
+  if (!parent)
     {
+      g_critical ("StTooltip is not parented");
       return;
     }
-  clutter_actor_get_size (stage, &stage_w, &stage_h);
+  clutter_actor_get_size (parent, &parent_w, &parent_h);
 
   /* make sure the tooltip is not off screen vertically */
-  if (tooltip_w > stage_w)
+  if (tooltip_x < 0)
     {
       tooltip_x = 0;
-      clutter_actor_set_width ((ClutterActor*) tooltip, stage_w);
     }
-  else if (tooltip_x < 0)
+  else if (tooltip_x + tooltip_w > parent_w)
     {
-      tooltip_x = 0;
-    }
-  else if (tooltip_x + tooltip_w > stage_w)
-    {
-      tooltip_x = (int)(stage_w) - tooltip_w;
+      tooltip_x = (int)(parent_w) - tooltip_w;
     }
 
   /* make sure the tooltip is not off screen horizontally */
-  if (tooltip_y + tooltip_h > stage_h)
+  if (tooltip_y + tooltip_h > parent_h)
     {
       priv->actor_below = TRUE;
-
-      /* re-query size as may have changed */
-      clutter_actor_get_preferred_height ((ClutterActor*) tooltip,
-                                          -1, NULL, &tooltip_h);
       tooltip_y = tip_area->y - tooltip_h;
     }
   else
@@ -366,7 +333,12 @@ st_tooltip_update_position (StTooltip *tooltip)
   /* calculate the arrow offset */
   priv->arrow_offset = tip_area->x + tip_area->width / 2 - tooltip_x;
 
-  clutter_actor_set_position ((ClutterActor*) tooltip, tooltip_x, tooltip_y);
+  /* Since we are updating the position out of st_widget_allocate(), we can't
+   * call clutter_actor_set_position(), since that would trigger another
+   * allocation cycle. Instead, we adjust the anchor position which moves
+   * the tooltip actor on the screen without changing its allocation
+   */
+  clutter_actor_set_anchor_point ((ClutterActor*) tooltip, -tooltip_x, -tooltip_y);
 }
 
 /**
@@ -411,32 +383,6 @@ static void
 st_tooltip_show (ClutterActor *self)
 {
   StTooltip *tooltip = ST_TOOLTIP (self);
-  ClutterActor *parent;
-  ClutterActor *stage;
-
-  parent = clutter_actor_get_parent (self);
-  stage = clutter_actor_get_stage (self);
-
-  if (!stage)
-    {
-      g_warning ("StTooltip is not on any stage.");
-      return;
-    }
-
-  /* make sure we're parented on the stage */
-  if (G_UNLIKELY (parent != stage))
-    {
-      g_object_ref (self);
-      clutter_actor_unparent (self);
-      clutter_actor_set_parent (self, stage);
-      g_object_unref (self);
-      parent = stage;
-    }
-
-  /* raise the tooltip to the top */
-  clutter_container_raise_child (CLUTTER_CONTAINER (stage),
-                                 CLUTTER_ACTOR (tooltip),
-                                 NULL);
 
   st_tooltip_update_position (tooltip);
 
@@ -477,7 +423,8 @@ st_tooltip_set_tip_area (StTooltip             *tooltip,
     g_boxed_free (CLUTTER_TYPE_GEOMETRY, tooltip->priv->tip_area);
   tooltip->priv->tip_area = g_boxed_copy (CLUTTER_TYPE_GEOMETRY, area);
 
-  st_tooltip_update_position (tooltip);
+  if (clutter_actor_get_stage (CLUTTER_ACTOR (tooltip)))
+    st_tooltip_update_position (tooltip);
 }
 
 /**

diff --git a/src/st/st-widget.c b/src/st/st-widget.c
index fe5d1b0c51e461f4db426213e4ac67709ca8e1ca..b9512ff194973f9ae186cbcf3848b7bb410a997f 100644 (file)
--- a/src/st/st-widget.c
+++ b/src/st/st-widget.c
@@ -59,6 +59,7 @@ struct _StWidgetPrivate
 
   gboolean      is_stylable : 1;
   gboolean      has_tooltip : 1;
+  gboolean      show_tooltip : 1;
   gboolean      is_style_dirty : 1;
   gboolean      draw_bg_color : 1;
   gboolean      draw_border_internal : 1;
@@ -124,6 +125,9 @@ static gboolean st_widget_real_navigate_focus (StWidget         *widget,
 
 static AtkObject * st_widget_get_accessible (ClutterActor *actor);
 
+static void st_widget_do_show_tooltip (StWidget *widget);
+static void st_widget_do_hide_tooltip (StWidget *widget);
+
 static void
 st_widget_set_property (GObject      *gobject,
                         guint         prop_id,
@@ -275,16 +279,8 @@ st_widget_dispose (GObject *gobject)
 
   if (priv->tooltip)
     {
-      ClutterContainer *parent;
-      ClutterActor *tooltip = CLUTTER_ACTOR (priv->tooltip);
-
-      /* this is just a little bit awkward because the tooltip is parented
-       * on the stage, but we still want to "own" it */
-      parent = CLUTTER_CONTAINER (clutter_actor_get_parent (tooltip));
-
-      if (parent)
-        clutter_container_remove_actor (parent, tooltip);
-
+      clutter_actor_destroy (CLUTTER_ACTOR (priv->tooltip));
+      g_object_unref (priv->tooltip);
       priv->tooltip = NULL;
     }
 
@@ -426,28 +422,28 @@ st_widget_parent_set (ClutterActor *widget,
 static void
 st_widget_map (ClutterActor *actor)
 {
-  StWidgetPrivate *priv = ST_WIDGET (actor)->priv;
+  StWidget *self = ST_WIDGET (actor);
 
   CLUTTER_ACTOR_CLASS (st_widget_parent_class)->map (actor);
 
-  st_widget_ensure_style ((StWidget*) actor);
+  st_widget_ensure_style (self);
 
-  if (priv->tooltip)
-    clutter_actor_map ((ClutterActor *) priv->tooltip);
+  if (self->priv->show_tooltip)
+    st_widget_do_show_tooltip (self);
 }
 
 static void
 st_widget_unmap (ClutterActor *actor)
 {
-  StWidgetPrivate *priv = ST_WIDGET (actor)->priv;
+  StWidget *self = ST_WIDGET (actor);
+  StWidgetPrivate *priv = self->priv;
 
   CLUTTER_ACTOR_CLASS (st_widget_parent_class)->unmap (actor);
 
-  if (priv->tooltip)
-    clutter_actor_unmap ((ClutterActor *) priv->tooltip);
-
   if (priv->track_hover && priv->hover)
-    st_widget_set_hover (ST_WIDGET (actor), FALSE);
+    st_widget_set_hover (self, FALSE);
+
+  st_widget_do_hide_tooltip (self);
 }
 
 static void notify_children_of_style_change (ClutterContainer *container);
@@ -1502,6 +1498,29 @@ st_widget_set_direction (StWidget *self, StTextDirection dir)
     st_widget_style_changed (self);
 }
 
+static void
+st_widget_ensure_tooltip_parented (StWidget *widget, ClutterStage *stage)
+{
+  StWidgetPrivate *priv;
+  ClutterContainer *ui_root;
+  ClutterActor *tooltip, *parent;
+
+  priv = widget->priv;
+
+  ui_root = st_get_ui_root (stage);
+
+  tooltip = CLUTTER_ACTOR (priv->tooltip);
+  parent = clutter_actor_get_parent (tooltip);
+
+  if (G_UNLIKELY (parent != CLUTTER_ACTOR (ui_root)))
+    {
+      if (parent)
+        clutter_container_remove_actor (CLUTTER_CONTAINER (parent), tooltip);
+
+      clutter_container_add_actor (ui_root, tooltip);
+    }
+}
+
 /**
  * st_widget_set_has_tooltip:
  * @widget: A #StWidget
@@ -1519,6 +1538,7 @@ st_widget_set_has_tooltip (StWidget *widget,
                            gboolean  has_tooltip)
 {
   StWidgetPrivate *priv;
+  ClutterActor *stage;
 
   g_return_if_fail (ST_IS_WIDGET (widget));
 
@@ -1534,17 +1554,18 @@ st_widget_set_has_tooltip (StWidget *widget,
       if (!priv->tooltip)
         {
           priv->tooltip = g_object_new (ST_TYPE_TOOLTIP, NULL);
-          clutter_actor_set_parent ((ClutterActor *) priv->tooltip,
-                                    (ClutterActor *) widget);
+          g_object_ref_sink (priv->tooltip);
+
+          stage = clutter_actor_get_stage (CLUTTER_ACTOR (widget));
+          if (stage != NULL)
+            st_widget_ensure_tooltip_parented (widget, CLUTTER_STAGE (stage));
         }
     }
-  else
+  else if (priv->tooltip)
     {
-      if (priv->tooltip)
-        {
-          clutter_actor_unparent (CLUTTER_ACTOR (priv->tooltip));
-          priv->tooltip = NULL;
-        }
+      clutter_actor_destroy (CLUTTER_ACTOR (priv->tooltip));
+      g_object_unref (priv->tooltip);
+      priv->tooltip = NULL;
     }
 }
 
@@ -1587,9 +1608,10 @@ st_widget_set_tooltip_text (StWidget    *widget,
   if (text == NULL)
     st_widget_set_has_tooltip (widget, FALSE);
   else
-    st_widget_set_has_tooltip (widget, TRUE);
-
-  st_tooltip_set_label (priv->tooltip, text);
+    {
+      st_widget_set_has_tooltip (widget, TRUE);
+      st_tooltip_set_label (priv->tooltip, text);
+    }
 }
 
 /**
@@ -1618,34 +1640,33 @@ st_widget_get_tooltip_text (StWidget *widget)
  * st_widget_show_tooltip:
  * @widget: A #StWidget
  *
- * Show the tooltip for @widget
+ * Force the tooltip for @widget to be shown
  *
  */
 void
 st_widget_show_tooltip (StWidget *widget)
 {
-  gfloat x, y, width, height;
-  ClutterGeometry area;
-
   g_return_if_fail (ST_IS_WIDGET (widget));
 
-  /* XXX not necceary, but first allocate transform is wrong */
-
-  clutter_actor_get_transformed_position ((ClutterActor*) widget,
-                                          &x, &y);
-
-  clutter_actor_get_size ((ClutterActor*) widget, &width, &height);
+  widget->priv->show_tooltip = TRUE;
+  if (CLUTTER_ACTOR_IS_MAPPED (widget))
+    st_widget_do_show_tooltip (widget);
+}
 
-  area.x = x;
-  area.y = y;
-  area.width = width;
-  area.height = height;
+static void
+st_widget_do_show_tooltip (StWidget *widget)
+{
+  ClutterActor *stage, *tooltip;
 
+  stage = clutter_actor_get_stage (CLUTTER_ACTOR (widget));
+  g_return_if_fail (stage != NULL);
 
   if (widget->priv->tooltip)
     {
-      st_tooltip_set_tip_area (widget->priv->tooltip, &area);
-      clutter_actor_show_all (CLUTTER_ACTOR (widget->priv->tooltip));
+      tooltip = CLUTTER_ACTOR (widget->priv->tooltip);
+      st_widget_ensure_tooltip_parented (widget, CLUTTER_STAGE (stage));
+      clutter_actor_raise (tooltip, NULL);
+      clutter_actor_show_all (tooltip);
     }
 }
 
@@ -1661,6 +1682,14 @@ st_widget_hide_tooltip (StWidget *widget)
 {
   g_return_if_fail (ST_IS_WIDGET (widget));
 
+  widget->priv->show_tooltip = FALSE;
+  if (CLUTTER_ACTOR_IS_MAPPED (widget))
+    st_widget_do_hide_tooltip (widget);
+}
+
+static void
+st_widget_do_hide_tooltip (StWidget *widget)
+{
   if (widget->priv->tooltip)
     clutter_actor_hide (CLUTTER_ACTOR (widget->priv->tooltip));
 }